fixed some linting issues and a missing variable declaration.

Nit: There's enough repeated code between the getters for broadcastIPv4 and localhostIPv4 that they should probably both call the same function just passing in an argument telling the function what environment variable to retrieve. (On the other hand, that can be refactored later.)

Should be good enough for ci now: https://ci.nodejs.org/job/node-test-pull-request/292/

LGTM if CI is happy

Lets get the refactor in while at it.

This doesn't seem to solve the problem with test-dgram-broadcast-multi-process.js on the 32-bit FreeBSD on the CI. Any ideas? https://ci.nodejs.org/job/node-test-commit-other/629/nodes=freebsd101-32/console https://ci.nodejs.org/job/node-test-commit-other/631/nodes=freebsd101-32/console

@Trott I emailed the servers admins a while ago; looks like the broadcast address is set incorrectly.

It does seem to fix it for FreeBSD 64-bit, so that's good!

And on 32-bit, although it doesn't make the test pass, it is an improvement. Without this change, test output was:

#[PARENT] sent 'First message to send' to 255.255.255.255:12346
#[PARENT] sent 'Second message to send' to 255.255.255.255:12346
#[PARENT] sent 'Third message to send' to 255.255.255.255:12346
#[PARENT] sent 'Fourth message to send' to 255.255.255.255:12346
#[PARENT] sendSocket closed
#[PARENT] Responses were not received within 5000 ms.
#[PARENT] Fail

With this change:

#[PARENT] sent 'First message to send' to 172.16.31.3:12346
#[CHILD] 93497 received 'First message to send' from {"address":"10.20.10.11","family":"IPv4","port":12346,"size":21}
#[PARENT] sent 'Second message to send' to 172.16.31.3:12346
#[CHILD] 93497 received 'Second message to send' from {"address":"10.20.10.11","family":"IPv4","port":12346,"size":22}
#[PARENT] sent 'Third message to send' to 172.16.31.3:12346
#[CHILD] 93497 received 'Third message to send' from {"address":"10.20.10.11","family":"IPv4","port":12346,"size":21}
#[PARENT] sent 'Fourth message to send' to 172.16.31.3:12346
#[CHILD] 93497 received 'Fourth message to send' from {"address":"10.20.10.11","family":"IPv4","port":12346,"size":22}
#[PARENT] sendSocket closed
#[PARENT] 93497 received 4 messages total.
#[PARENT] Responses were not received within 5000 ms.
#[PARENT] Fail

That's several steps in the right direction.

@Trott refactor lgtm as well?

@jbergstroem Yes, LGTM with the refactor.

I prefer writing the function after the variable declaration but I am okay with landing as it is. LGTM

I'll see if there's anything we can do in the host machine about broadcast before merging this so we actually "fix" the issue. Otherwise we might have to work around it in our CI.

Host rebooted, should've fixed the broadcast. I think we have a winner: https://ci.nodejs.org/job/node-test-pull-request/300/

@thefourtheye I'll move the function below.

@Trott can you confirm that it closes #2472 ?

https://ci.nodejs.org/job/node-test-commit-other/647/

It's actually worse now. Now both 64-bit and 32-bit FreeBSD are failing and the test-dgram-multicast-multi-process.js is failing on them as well as test-dgram-broadcast-multi-process.js.

On the upside, they are getting a specific broadcast address that isn't 255.255.255.255.

That's unfortunate. This PR should be pretty much ok to land since it at least gives us control. I'll avoid adding "fixed" then.

Yeah, actually, even though the tests failed, they're failing further along than before, so it's progress!

LGTM

Just wanted to update. We're still stuck with figuring out why broadcasting from jails aren't working as intended. This PR will work just fine but the test will still fail [for other reasons]. Not sure we should merge this just yet since it might be avoided by finding another solution in CI-land.

Thinking we might skip this -- ref nodejs/build#228.

@jbergstroem ... where are we at on this?

We still have the issue but are migrating away from jails in ci. I'm inclined to close and reopen if some other freebsd user can help debug (I didn't have access to host).